Sorry, I don't understand the motivation of this PR at all.

I'm passing in $_SERVER as I need some environment variables to be available during CLI::configure - in particular the HOME. As this is in the CLI calling wrapper script, it uses the array that is available. Could probably also have been $_ENV.

Anyway, your PR would break that. It creates a global dependency into CLI for no reason and takes away the option to arbitrarily pass in values. So, it would worsen things, wouldn't it?

Secondly, I don't get your point on arguments. The argument parsing is already available. I don't understand what you are trying to accomplish here?

Also, your code uses the short-array-syntax. That won't work on PHP 5.3.

Okay, I get it. The ezcConsoleInput could use that. But we do not need a new Parameter for that. We just need to pass it from the $env we already have.

Changed: https://github.com/theseer/Autoload/blob/master/src/CLI.php#L82

@theseer thank you for reviewing this PR and your comments!

Sorry, I don't understand the motivation of this PR at all.

I am using your library from another script. Hence, I'm not calling e.g. phpab.php -- some command options, but rather like so:

$factory = new \TheSeer\Autoload\Factory();
$rc = $factory->getCLI()->run(array('--', 'some', 'command', 'options'));

I'm passing in $_SERVER as I need some environment variables to be available during CLI::configure - in particular the HOME. As this is in the CLI calling wrapper script, it uses the array that is available. Could probably also have been $_ENV.

Yes, and this is still possible (as the second parameter to run(), though). And it is now optional and will default to $_SERVER in CLI::configure():

That's why I have removed it in the calling scripts. However, I can also re-add them like so:

$rc = $factory->getCLI()->run(null, $_SERVER);

Anyway, your PR would break that. It creates a global dependency into CLI for no reason and takes away the option to arbitrarily pass in values. So, it would worsen things, wouldn't it?

I'm not quite sure what you mean by a "global dependency". But as laid out above, arbitrary values can still be passed.

Also, your code uses the short-array-syntax. That won't work on PHP 5.3.

Happy to fix those.

Changed: https://github.com/theseer/Autoload/blob/master/src/CLI.php#L82

I see.

Well, but that would mean that I need to do something like this, right?

$factory = new \TheSeer\Autoload\Factory();
$env = [
  'HOME' => $_SERVER['HOME'] ?? null,
  'HOMEDRIVE' => $_SERVER['HOMEDRIVE'] ?? null,
  'HOMEPATH' => $_SERVER['HOMEPATH'] ?? null,
  'argv' => ['--', 'some', 'command', 'options',]
]
$rc = $factory->getCLI()->run($env);

This is of course much more complicated, than just adding my options:

$factory = new \TheSeer\Autoload\Factory();
$rc = $factory->getCLI()->run(['--', 'some', 'command', 'options',]);

Also, I'm not sure that with your solution the $env['argv]-key needs to exist. I suspect the following would break the code:

$factory = new \TheSeer\Autoload\Factory();
$env = [
  'HOME' => $_SERVER['HOME'] ?? null,
]
$rc = $factory->getCLI()->run($env);

As ?: does not work on non-existent keys, does it?

I'm not quite sure what you mean by a "global dependency". But as laid out above, arbitrary values can still be passed.

The very concept of a "super globals" was a design mistake and should never have made it into PHP as it leaks global state. Which is bad. Hence me passing in the $_SERVER array. You are free to provide any type of array you feel like when using it. It should contain the required environment variables that can be expected in CLI context.

Furthermore I believe that you now pass an array (even just an empty one) to $input->process(). However, this would prevent \ezcConsoleInput::process() form actually using $_SERVER['argv'], as isset() will always return true.

That's correct and intended. I despise access to global state.

That's correct and intended. I despise access to global state.

I see. So you prevent doing it by doing it yourself. 😉

Anyway. Thanks for change. 🙏

Well, but that would mean that I need to do something like this, right?

$factory = new \TheSeer\Autoload\Factory();
$env = [
  'HOME' => $_SERVER['HOME'] ?? null,
  'HOMEDRIVE' => $_SERVER['HOMEDRIVE'] ?? null,
  'HOMEPATH' => $_SERVER['HOMEPATH'] ?? null,
  'argv' => ['--', 'some', 'command', 'options',]
];
$rc = $factory->getCLI()->run($env);

Yes and No.

$rc = $factory->getCLI()->run(array_merge($_SERVER, array('argv' => ....));

Would be enough if you're in CLI context yourself.
If not, you have to get the values from somewhere.

As ?: does not work on non-existent keys, does it?

Valid point. Fixed.

That's correct and intended. I despise access to global state.

I see. So you prevent doing it by doing it yourself. 😉

Not sure how you mean this. Of course one has to get the Information from somewhere, but the only time I access it, is in the global state of the bootstrap script. The actual application never sees the global state, as it should be.